-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add setup check to verify that the used DB version is still supported… #25016
Conversation
ba0724b
to
efde9a7
Compare
|
||
public function check() { | ||
switch (get_class($this->connection->getDatabasePlatform())) { | ||
case MySQL80Platform::class: # extends MySQL57Platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left all the platforms in here to have the full overview (except DB2 or MSSQLServer as we don't support them at all)
ff94601
to
58adf13
Compare
Ready for review :) |
58adf13
to
834d8a5
Compare
/backport to stable20 |
$version = strtolower($row['Value']); | ||
|
||
if (strpos($version, 'mariadb') !== false) { | ||
if (version_compare($version, '10.4', '<')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (version_compare($version, '10.4', '<')) { | |
if (version_compare($version, '10.2', '<')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But 10.4 is the mysql8 compatible version?
Ref https://github.com/nextcloud/server/blob/stable16/lib/private/Setup/MySQL.php#L48-L56
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our docs say, that 10.2 works and CI runs against 10.1 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to 10.2 for now. We can raise it later again, okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10.2 is still supported so yeah :)
… in the next major release Signed-off-by: Morris Jobke <hey@morrisjobke.de>
834d8a5
to
bffa61f
Compare
@rullzer @ChristophWurst Ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
But didn't test
… in the next major release